Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Add motd to autohost and challenge #3648

Closed
wants to merge 3 commits into from

Conversation

CupnPlateGames
Copy link
Contributor

A basic motd to display in the chatbox for autohost and challenges.

Would multilingual motd be appreciated? I think I've understood how to do it but it seems to require to update the serialization of the game options to add all motds and let the client pick the right one to display.

@CupnPlateGames
Copy link
Contributor Author

This is how multilingual motd would work:

"motd": {
    "default": "The fallback message when the motd in the client's language is not defined.",
    "ar_SA": "Translated message in Arabic",
    "bg": "Translated message in Bulgarian",
    [...]
}

Only the "default" entry is required. For ease-of-use and compatibility if the simple way is merged first, if "motd" is a string it would be used as the default one without any language-specific motd.

In multiopt.cpp the motds are sent along other game options. It would be serialized with the number of motds (NETuint8_t) and a couple of strings for each motd (NETstring(locale, 8) and NETstring(motd, 256)). There are 35 languages defined in i18n.cpp, maybe the number of motds can be capped to 36 (getLocales() size), so bloating up to nearly 10kB when abusing the network.

When received, the client will pick the motd matching their language, or use the default one.

So before coding it, I'd like to have some feedback if this way of doing it is acceptable. It will also be inconsistent with challenge name and description which are not translated (yet).

@past-due
Copy link
Member

past-due commented Feb 24, 2024

In multiopt.cpp the motds are sent along other game options. It would be serialized with the number of motds (NETuint8_t) and a couple of strings for each motd (NETstring(locale, 8) and NETstring(motd, 256)). There are 35 languages defined in i18n.cpp, maybe the number of motds can be capped to 36 (getLocales() size), so bloating up to nearly 10kB when abusing the network.

Instead of sending it along with the game options, it should be a separate one-time message sent post-join (which seems to be what this PR currently does). Especially given the size.

(And, since it might be useful in other cases, we could add this as a new NET_I18NTEXTMSG type, restricted to being sent by the host. Also, the fallback should probably always be English - to match the game’s behavior. So perhaps “en” could be required if supplying it as a dictionary, and any other languages could be optional.)

As long as they are only sent once, I think we can stomach an (optional) couple of KB on join (if a host really wants to translate their MOTD for all languages - which I suspect most will only do a few anyway).

@CupnPlateGames
Copy link
Contributor Author

Thank you for the feedback. I will take some time to implement all of this but it wouldn't break the current code structure. It could still be merged in the current state if i18n is not ready.

Also, the fallback should probably always be English - to match the game’s behavior.

As the message is user-provided from hosters, there's nothing preventing hosters to use their native language in the English entry as the default one (when English is not provided). But duplication with the actual language code is dumb when thinking about it.

I'll go for English-as-default at first for simplicity and maybe add a "default": "default locale" instead of the default message so the "default" is not mandatory for that very specific use-case and keep it simple for the regular cases.

@CupnPlateGames
Copy link
Contributor Author

It's done for the main parts. But it doesn't work with the system locale yet. And I haven't checked how to restrict message types only for the host.

I would appreciate a code review, I don't do much C++ and a lot of this was done through trial and errors.

@CupnPlateGames CupnPlateGames changed the title Add motd to autohost and challenge [WIP] Add motd to autohost and challenge Mar 24, 2024
@KJeff01 KJeff01 added PR: Needs Review Check if this PR is still relevant or needs minor tweaks to be ready to merge PR: Limbo Work that was never finished and may have significant conflicts. PRs will be slated for closure. and removed PR: Needs Review Check if this PR is still relevant or needs minor tweaks to be ready to merge labels Oct 2, 2024
@KJeff01 KJeff01 closed this Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Limbo Work that was never finished and may have significant conflicts. PRs will be slated for closure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants